-
Notifications
You must be signed in to change notification settings - Fork 29
Add more state pictograms #1271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/state-pictogram-tooltips
Are you sure you want to change the base?
Add more state pictograms #1271
Conversation
7d04cb3 to
4c6da51
Compare
| if ($icon === null) { | ||
| if (! $item->notifications_enabled) { | ||
| $icon = new Icon(Icons::NO_NOTIFICATIONS, ['title' => $this->translate('Notifications disabled')]); | ||
| } elseif (! $item->active_checks_enabled) { | ||
| $icon = new Icon(Icons::NO_ACTIVE_CHECKS, ['title' => $this->translate('Active checks disabled')]); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These icons are still missing in the specific column view list (e.g., URL: icingadb/hosts?columns=name). These should be added to the State::getIcon() method to cover all use cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The columns notifications_disabled and active_checks_disabled do not come from the state, but the corresponding host/service item. Would you still add this to State::getIcon()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather add the logic to StateRowItem::assembleVisual() directly to access the item's columns.
6a39bfc to
428345d
Compare
4c6da51 to
ce7f0d2
Compare
ce7f0d2 to
e10c9f4
Compare
b7126c6 to
541ce9b
Compare
Now also add pictograms to state ball in the following cases: - Disabled active checks - Disabled notifications
e10c9f4 to
543c756
Compare
Now also add pictograms to state ball in the following cases:
If both active checks and notifications are disabled, the “notifications disabled” icon is shown. This is because results may still arrive from passive checks, but they cannot trigger notifications when notifications are disabled.
Resolves #1097